Skip to content

Conversation

@mabaasit
Copy link
Collaborator

This test has been flaking a lot due to modal not closing as we press the escape.

Description

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@github-actions github-actions bot added the fix label Jan 14, 2025
@mabaasit mabaasit added the no release notes Fix or feature not for release notes label Jan 14, 2025
@mabaasit mabaasit changed the title fix(e2e-tests): close the focus-mode modal fix(e2e-tests): close the focus-mode modal COMPASS-8820 Jan 15, 2025
Comment on lines 1339 to 1343
// As the tooltip disappeared at this point, the previousButton still
// has the focus. Pressing escape twice, should close the modal.
await browser.keys([Key.Escape]);
await sleep(100);
await browser.keys([Key.Escape]);
Copy link
Collaborator

@gribnoysup gribnoysup Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there is a way for us to do it without an arbitrary timeout, like removing the focus from the button? Also just generally wondering why button is swallowing the Esc keypress in this case, is this a bug? Nevermind this part, I tried it out and can see how this happens, just wasn't clear for me from the comment in the code: tooltip shows and this first esc press closes the tooltip.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, i can try to remove the focus from it. I think it has to do with tooltip. I tried to re-create this here, with two different versions - one with tooltip and one without tooltip and the one with tooltip has this issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think without complicating it even more, i'd do as we do at other places, close the modal using its close button.

@mabaasit mabaasit merged commit 0a65dbe into main Jan 16, 2025
28 of 30 checks passed
@mabaasit mabaasit deleted the flakes-fix branch January 16, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants